feat(db): add property registry table#1779
feat(db): add property registry table#1779kaligrafy wants to merge 4 commits intochairemobilite:mainfrom
Conversation
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/transition-backend/src/models/capnpDataModel/capnpFiles/dataSource.capnp">
<violation number="1" location="packages/transition-backend/src/models/capnpDataModel/capnpFiles/dataSource.capnp:31">
P1: Changing the numeric tag for the existing `unknown` enum value breaks backwards compatibility with previously serialized data. Keep `unknown` at @14 and assign the new value a new tag.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/transition-backend/src/models/capnpDataModel/capnpFiles/dataSource.capnp
Show resolved
Hide resolved
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new "propertyRegistry" data source across the codebase: a DB enum migration and a migration that creates tr_property_registry with PostGIS geography columns and indexes; a geometry helper and geometry unit tests; a property registry TypeScript model and tests; a new DB queries module implementing CRUD, geo-queries, and GeoJSON output; updates to default query generics (introducing Idable); Cap'n Proto and Rust enum updates for the new data source; multiple tests exercising DB and model logic; and minor package dependency edits. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
7daf57e to
5c60929
Compare
tahini
left a comment
There was a problem hiding this comment.
Bon, j'avais des commentaires pending...
Aussi, tel que discuté hors ligne, ce serait bien d'avoir les requêtes qui seront associées à cette table, afin de pouvoir mieux juger de l'utilisation qui en sera faite et, donc, de la validité des index/champs proposés.
...ages/chaire-lib-backend/src/models/db/migrations/20260212090800_createPropertyRegistryTbl.ts
Outdated
Show resolved
Hide resolved
...ages/chaire-lib-backend/src/models/db/migrations/20260212090800_createPropertyRegistryTbl.ts
Outdated
Show resolved
Hide resolved
...ages/chaire-lib-backend/src/models/db/migrations/20260212090800_createPropertyRegistryTbl.ts
Outdated
Show resolved
Hide resolved
7622253 to
b26c01b
Compare
|
J'ai ajouté les requêtes et les tests associés. |
b26c01b to
37f693a
Compare
37f693a to
ed5e928
Compare
|
@kaligrafy Can you compare with the work that Paul did? See how close you are in the table structure? (If we can reconcile both that could be interesting in the future) |
ed5e928 to
bc713a0
Compare
The work of Paul is specified for Quebec only. Mine is for any property registry databse and should be generic to most countries. All the columns he used in his projects are indeed included in the table schema implemented here. |
bc713a0 to
5cff0b4
Compare
5cff0b4 to
3f867a2
Compare
tahini
left a comment
There was a problem hiding this comment.
qq petits commentaires et réponses, je ferai un review plus complet demain.
...ages/chaire-lib-backend/src/models/db/migrations/20260212090800_createPropertyRegistryTbl.ts
Show resolved
Hide resolved
3f867a2 to
d1bf5e2
Compare
1518510 to
b5dc442
Compare
packages/chaire-lib-backend/src/models/db/__tests__/propertyRegistry.db.test.ts
Show resolved
Hide resolved
packages/chaire-lib-backend/src/models/db/__tests__/propertyRegistry.db.test.ts
Outdated
Show resolved
Hide resolved
packages/chaire-lib-backend/src/models/db/__tests__/propertyRegistry.db.test.ts
Outdated
Show resolved
Hide resolved
packages/chaire-lib-backend/src/models/db/propertyRegistry.db.queries.ts
Outdated
Show resolved
Hide resolved
packages/chaire-lib-common/src/services/propertyRegistry/__tests__/PropertyRegistry.test.ts
Outdated
Show resolved
Hide resolved
packages/chaire-lib-common/src/services/propertyRegistry/PropertyRegistry.ts
Outdated
Show resolved
Hide resolved
packages/chaire-lib-common/src/services/propertyRegistry/PropertyRegistry.ts
Outdated
Show resolved
Hide resolved
packages/chaire-lib-common/src/services/propertyRegistry/PropertyRegistry.ts
Outdated
Show resolved
Hide resolved
b5dc442 to
b83c802
Compare
b83c802 to
26f5760
Compare
The util function does convert geojson input into a postgis raw query string. If the geojson input is undefined, it will return undefined. This conversion operation is repeated in every db insert query where at least one column is of type geom or geog. Now, we will be able to use this util instead.
Data source type did not include property registry The capnp enum was also updated. Update to capnp schema used services/json2capnp/transition_capnp_data/src/capnp/_generating_classes.txt
Accept Idable instead of GenericObject so we can input custom objects with at least an id (number of string) Right now, the default queries only accept GenericObject, but new classes will not have some of the attributes in GenericObject. To make any object with at least an id to be able to use these query templates, we need to change the input type to a simple Idable.
Includes: - db queries - associated types - tests New package: - iso-3166-ts (for the country field so we make it standard two-letter country code) completes step 1 of chairemobilite#1691
26f5760 to
6799be2
Compare
Add two migrations to chaire-lib-backend:
The table supports international property/cadastral data with:
Updates to capnp schema for data sources have also been updated. However, we should not need any capnp schema for data sources and zones, so they will be removed in the future. See #1778
See: https://github.com/chairemobilite/transition/wiki/Documentation-%E2%80%90-Property-Registry
Also see: #1691 (closes step 1)
Summary by CodeRabbit